Skip to content

Conversation

@akroviakov
Copy link
Contributor

This PR adds XeVM load/store/prefetch operations and their conversion patterns to llvm. We follow the Triton approach with slight changes (e.g., the cache attribute is handled differently).

@akroviakov akroviakov force-pushed the akroviak/xevm-conversions branch from e0a6fda to d59728f Compare December 15, 2024 16:15
@akroviakov akroviakov force-pushed the akroviak/xevm-conversions branch 2 times, most recently from 3845562 to 3315f73 Compare December 16, 2024 15:55
Copy link
Contributor

@kurapov-peter kurapov-peter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good with the map for cache levels and the width/pitch/height description (not necessary atm)

@akroviakov akroviakov force-pushed the akroviak/xevm-conversions branch 3 times, most recently from a96e86c to 349bf35 Compare December 17, 2024 12:58

namespace {

enum CacheLevel { L1, L2, L3 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
enum CacheLevel { L1, L2, L3 };
enum class CacheLevel { L1, L2, L3 };

getCacheControlMetadata(ConversionPatternRewriter &rewriter, OpType op,
const bool isLoad, const std::string &chip) {
if ((op.getL1CacheControlAttr() ==
xevm::L1StoreCacheControlAttr::get(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
xevm::L1StoreCacheControlAttr::get(
L1StoreCacheControlAttr::get(

You may omit the namespace here and below or remove using namespace xevm, seems it's never used.

static LLVM::CallOp createDeviceFunctionCall(
ConversionPatternRewriter &rewriter, StringRef funcName, Type retType,
ArrayRef<Type> argTypes, ArrayRef<Value> args,
mlir::ArrayRef<std::pair<unsigned, mlir::StringRef>> paramAttrs,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
mlir::ArrayRef<std::pair<unsigned, mlir::StringRef>> paramAttrs,
ArrayRef<std::pair<unsigned, StringRef>> paramAttrs,

To be consistent with other args.

return llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(type, val));
}

static LogicalResult handleDecorationCacheControl(Operation *op,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the op argument is never used.

@akroviakov akroviakov force-pushed the akroviak/xevm-conversions branch from 349bf35 to c3e7f25 Compare December 17, 2024 15:28
@kurapov-peter kurapov-peter merged commit 9dd4e60 into main Dec 18, 2024
6 checks passed
@kurapov-peter kurapov-peter deleted the akroviak/xevm-conversions branch December 18, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants